feat: get clang-tidy fixes regardless of tidy-review value by 2bndy5 · Pull Request #354 · cpp-linter/cpp-linter-rs

2bndy5 · 2026-06-12T01:58:55Z

This is another step toward presenting auto-fixed patches for consumers to commit/push.

This basically does what --tidy-review conditionally did before. But now it is done every time clang-tidy is invoked.

Even if not using the PR review feature, this allows us to boast if a clang-tidy note/warning can be auto-fixed. For example, the new thread comment may contain the following for something that can be auto-fixed:

src/demo/demo.cpp:10:3 note: [readability]
⚡ auto-fix available

{rationale}

{concerned code snippet if any}

Summary by CodeRabbit

New Features
- Auto-fix availability indicator added to clang-tidy suggestions in review comments.
Bug Fixes
- More reliable caching and restoration of patched files to improve suggestion accuracy and stability.
- Suggestion generation now consistently reads cached patched content.
Tests
- Tests updated to use isolated temporary workspaces and cover the new patch/restore behavior.
Documentation
- Spellcheck allowlist updated to include RAII.

This is another step toward presenting auto-fixed patches for consumers to commit/push. This basically does what `--tidy-review` conditionally did before. But now it is done every time clang-tidy is invoked. Even if not using the PR review feature, this allows us to boast if a clang-tidy note/warning can be auto-fixed. For example, the new thread comment may contain the following for something that can be auto-fixed: - **src/demo/demo.cpp:10:3** note: [readability] :zap: auto-fix available > <rationale> {concerned code snippet if any}

codecov · 2026-06-12T02:18:43Z

Codecov Report

❌ Patch coverage is 84.31373% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.83%. Comparing base (793c310) to head (7079384).

Files with missing lines	Patch %	Lines
cpp-linter/src/clang_tools/clang_tidy.rs	76.81%	16 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #354      +/-   ##
==========================================
+ Coverage   92.74%   92.83%   +0.09%     
==========================================
  Files          22       23       +1     
  Lines        3403     3461      +58     
==========================================
+ Hits         3156     3213      +57     
- Misses        247      248       +1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai · 2026-06-12T04:30:21Z

Warning

Review limit reached

@2bndy5, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 35 minutes and 54 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 172982af-7170-4489-9502-69f156d653df

📥 Commits

Reviewing files that changed from the base of the PR and between d7902c3 and 7079384.

📒 Files selected for processing (4)

cpp-linter/src/clang_tools/clang_tidy.rs
cpp-linter/src/lib.rs
cpp-linter/src/rest_client.rs
cpp-linter/src/run.rs

Walkthrough

TidyAdvice.patched is now a PathBuf to a cached .tidy file. parse_tidy_output returns a Vec. run_clang_tidy always runs clang-tidy --fix-errors, caches patched files under CACHE_DIR, restores repo files via a RestoreOnDrop guard, and downstream code reads patched content from disk; UI shows auto-fix markers.

Changes

TidyAdvice File-Caching Refactor

Layer / File(s)	Summary
TidyAdvice contract and parse refactor `cpp-linter/src/clang_tools/clang_tidy.rs`	`TidyAdvice.patched` changes from `Option<Vec<u8>>` to `PathBuf`. Module imports updated. `parse_tidy_output` now returns `Result<Vec<TidyNotification>, ClangCaptureError>` and tests updated to inspect the returned Vec.
File caching and restoration in run_clang_tidy `cpp-linter/src/clang_tools/clang_tidy.rs`	Always adds `--fix-errors`, reads original file into `RestoreOnDrop`, runs clang-tidy to patch in-place, writes patched output to `CACHE_DIR` with `.tidy` extension, restores original repo file, disarms the guard, and sets `file.tidy_advice` with the cached path. Tests added/updated for stdout parsing and RestoreOnDrop behavior.
Patched content reading in make_suggestions_from_patch `cpp-linter/src/common_fs.rs`	Now loads patched content via `fs::read_to_string(&advice.patched)` (cached PathBuf) and passes it to `make_patch` for suggestions.
Auto-fix display and test data alignment `cpp-linter/src/rest_client.rs`	`make_tidy_comment` appends an auto-fix zap indicator when a note's line is in `fixed_lines`. Test data updated to set `TidyNotification.patched` to `PathBuf::new()` instead of `None`.
Test harness and temp workspace `cpp-linter/src/run.rs`	`mod test` made `pub(crate)` and adds `setup_tmp_workspace()` to copy demo assets into a temp dir; async tests updated to use per-test `--repo-root` pointing at the temp workspace.
Spell-check config `cspell.config.yml`	Add `RAII` to allowed words.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

cpp-linter/cpp-linter-rs#190: Modifies parse_tidy_output regex and note-header parsing logic, which this PR refactors further to change return types and responsibility boundaries.
cpp-linter/cpp-linter-rs#347: Aligns with this PR's pattern of storing clang-format/clang-tidy patched output as PathBuf cached on disk rather than in-memory bytes.
cpp-linter/cpp-linter-rs#77: Previously modified FileObj::make_suggestions_from_patch and diff-aware gating; closely related to how suggestions should be generated from cached patched content.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: obtaining clang-tidy fixes unconditionally, regardless of the tidy-review flag setting.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch tidy-fixes-always

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

try to restore scanned file's contents upon error propagation. add more test isolation to prevent race conditions with shared test asserts

This is a culmination of - #354 - #347 It it the last step needed before providing a patch that consumers can push to auto-fix lints. # Intended flow of data 1. run clang-tidy before running clang-format 2. after running clang-tidy save the patched file to cache 3. after running clang-format: - check for clang-tidy patch in cache. - if cached patch is present, run clang-format on the clang-tidy patch. This step includes formatting lines that clang-tidy changed and lines that clang-format changed. - if cached patch is not present (clang-tidy was not run on the file), then cache the clang-format fixes instead. - `--lines-changed-only` is respected, but this may need tweaking in the future. 4. when creating a PR review, only add hunks that are not present in the review comments. I added a parameter to also calculate (and display) how many review comments were reused in previous PR reviews.

This is a culmination of - #354 - #347 It it the last step needed before providing a patch that consumers can push to auto-fix lints. # Intended flow of data 1. run clang-tidy before running clang-format 2. after running clang-tidy save the patched file to cache 3. after running clang-format: - check for clang-tidy patch in cache. - if cached patch is present, run clang-format on the clang-tidy patch. This step includes formatting lines that clang-tidy changed and lines that clang-format changed. - if cached patch is not present (clang-tidy was not run on the file), then cache the clang-format fixes instead. - `--lines-changed-only` is respected, but this may need tweaking in the future. 4. when creating a PR review, only add hunks that are not present in the review comments. I added a parameter to also calculate (and display) how many review comments were reused in previous PR reviews. Adjusted tests accordingly.

This is a culmination of - #354 - #347 It it the last step needed before providing a patch that consumers can push to auto-fix lints. # Intended flow of data 1. run clang-tidy before running clang-format 2. after running clang-tidy save the patched file to cache 3. after running clang-format: - check for clang-tidy patch in cache. - if cached patch is present, run clang-format on the clang-tidy patch. This step includes formatting lines that clang-tidy changed and lines that clang-format changed. - if cached patch is not present (clang-tidy was not run on the file), then cache the clang-format fixes instead. - `--lines-changed-only` is respected, but this may need tweaking in the future. 4. when creating a PR review, only add hunks that are not present in the review comments. I added a parameter to also calculate (and display) how many review comments were reused in previous PR reviews. If PR review is a summary only, then the patch will include all fixes that would have been posted in the diff. Adjusted tests accordingly.

This is a culmination of - [x] #354 - [x] #347 As this builds on the plan discussed in cpp-linter/cpp-linter#82, it it the last step needed before providing a patch that consumers can push to auto-fix lints. ## Intended flow of data 1. run clang-tidy before running clang-format 2. after running clang-tidy save the patched file to cache 3. after running clang-format: - check for clang-tidy patch in cache. - if cached patch is present, run clang-format on the clang-tidy patch. This step includes formatting lines that clang-tidy changed and lines that clang-format changed. - if cached patch is not present (clang-tidy was not run on the file), then cache the clang-format fixes instead. - `--lines-changed-only` is respected, but this may need tweaking in the future. 4. when creating a PR review, - only add hunks to the patch that are not present in the review comments. - I added a parameter to also calculate (and display) how many review comments were reused in previous PR reviews. - PR review summaries are worded better if the env var `CPP_LINTER_PR_REVIEW_SUMMARY_ONLY = 1`. If PR review is a summary only, then the patch will include all fixes that would have been posted in the diff. --- Adjusted tests accordingly.  ## Summary by CodeRabbit * **Bug Fixes** * Improved formatting/tidy patch handling to generate more accurate review diffs and line-range suggestions. * Refined how review summaries are produced (including “summary-only” behavior and reused-review messaging). * Improved review comment metadata when line ranges are identical. * **Tests** * Expanded diff and three-way diff coverage. * Updated review-body expectation logic to match the new summary rules. * **Chores** * Updated documentation dependency. * Refined caching and output processing for more reliable review generation.

resolves #376 This is the result of - #358 - #354 - #347 If there are any auto-fixes available, then a patch full of them is created and its path is set to a new output `fix-patch-path` output variable. If there were no auto-fixes, then the output variable is not set. Remember, an unset output variable is treated as an empty string (a false value) in `steps.linter.outputs.fix-patch-path` context.  ## Summary by CodeRabbit ## Release Notes * **New Features** * Auto-fix patches are aggregated into a single unified patch file in the repository cache directory. * CI/CD now receives an optional `fix-patch-path` output (with normalized forward slashes) pointing to the generated patch when present. * Cached patch aggregation is recorded consistently when producing suggestions. * **Bug Fixes** * Clear any previously generated unified patch before starting a new run to prevent stale outputs. * **Tests** * Updated test inputs to ignore `build/**` and added validation for the generated patch file contents.

2bndy5 added the enhancement New feature or request label Jun 12, 2026

2bndy5 changed the title ~~feat: always get clang-tidy fixes~~ feat: get clang-tidy fixes regardless of tidy-review value Jun 12, 2026

2bndy5 marked this pull request as ready for review June 12, 2026 04:23

This comment was marked as resolved.

Sign in to view

implement drop guard and isolate test assets

d7902c3

try to restore scanned file's contents upon error propagation. add more test isolation to prevent race conditions with shared test asserts

This comment was marked as resolved.

Sign in to view

nit picky changes

7079384

2bndy5 merged commit 41fcd9e into main Jun 12, 2026
73 checks passed

2bndy5 deleted the tidy-fixes-always branch June 12, 2026 09:02

2bndy5 mentioned this pull request Jun 14, 2026

feat: merge review suggestions from both clang tools #358

Merged

2 tasks

2bndy5 mentioned this pull request Jun 20, 2026

feat: add fix-patch-path output variable #378

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: get clang-tidy fixes regardless of tidy-review value#354

feat: get clang-tidy fixes regardless of tidy-review value#354
2bndy5 merged 3 commits into
mainfrom
tidy-fixes-always

2bndy5 commented Jun 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

codecov Bot commented Jun 12, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 12, 2026 •

edited

Loading

Review limit reached

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

2bndy5 commented Jun 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

codecov Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

2bndy5 commented Jun 12, 2026 •

edited by coderabbitai Bot

Loading

codecov Bot commented Jun 12, 2026 •

edited

Loading

coderabbitai Bot commented Jun 12, 2026 •

edited

Loading